-
Notifications
You must be signed in to change notification settings - Fork 95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for wide characters to fill
functions
#552
Add support for wide characters to fill
functions
#552
Conversation
For tests you could just update the user guide to include some emojis. The user guide code blocks are run through test-doc-blocks. |
Don't be too worried about CI failures, especially on Windows. That platform is still flakey on CI. Just rerun failed jobs. |
Thanks @lread ! I've added some emojis to the examples as suggested, but is it enough? As far as I'm aware there is no check that the "text" was actually typed correctly (or is there?) |
Ah yeah, good point! Could add in a fetch for the element value. Also: thanks for fixing various typos as you notice them. I am the king of typos. |
Windows CI is horribly flakey, as a Window guy, if you have any insights let us know. |
@lread I've just uploaded a new test that follows the same pattern as
my pleasure :)
I can take a look, but I don't promise anything 😅 I'm using Anyway, I'll try to run the "windows" tests on my local PC and see if there are any failures. |
Interesting! If you have trouble getting these to pass, an alternative might be to try updating user guide and add something like so: (e/get-element-value driver :uname)
;; => "my fancy expected emojis" Would that work? (this would become an assertion that test-doc-blocks runs)
Ya I suspect it is GitHub Actions (it stays up to date with browsers and drivers), I've not had much failure when running in my Windows VM. But if you take a peek, thanks in advance. |
oh yes that's much nicer (and easier) than what I had done :) updated as suggested |
Hmmm... I am trying this out in my REPL. |
Ah we seem to maybe have the same codepoint issue for |
Ah, maybe sending emoji chars with ChromeDriver does not work?: |
I think chrome only currently supports Unicode in the Basic Multilingual Plane. So, maybe ok. Different drivers/browsers have different limitations. Could run this test under Firefox only. I expect Microsoft Edge would be the same as it is based on the same code as Chrome. @tupini07, I can take a peek at this if you like, I have all the necessary VMs setup to poke around. |
@lread thanks for the investigation! Yeah it does sound like chromedriver has a limitation, interesting. I'm not home now so I can't test on my end wether this happen on my chrome as well. And yes sure! If you want to look into it that would be great! As I said I'm not homj right now, and probably won't be able to work on this until tomorrow night :) |
Ya, there is really no big rush. I can take a peek maybe tomorrow. |
Thank goodness for VMs and remote REPLs! My findings across OSes are:
I'll likely follow up tomorrow. |
Apply codepoint handling fix to input fill fns in general. Update user guide to talk about limitations. Add some unit tests to for fuller browser coverage.
Ok @tupini07, what do you think of my small tweaks to your fine effort? |
fill-human
fill
functions
@lread wonderful! Thanks a lot for your help 🤗 it would have taken me ages to work out how to properly test this and all the other changes you did. Also, great catch with this needing to be added to This PR has my name but basically you were the one who implemented everything 😅 |
Thanks for reviewing @tupini07, I shall merge. |
Addendum for #552 Add jdk variants for CI. Default to jdk 21 but also sanity test on jdks 8, 11 and 17.
Previously, when
api/fill-human
was asked to write text that contained emojis (or other "wide" characters) it would end up writing??
to the input field instead of the actual emoji. The reason for this is that it was just iterating over every character in the string instead of over the codepoints.This PR adds support for wide characters to
api/fill-human
by iterating over its codepoints.Closes #551
wokring-codepoints.webm
Please complete and include the following checklist:
I have read CONTRIBUTING and the Etaoin Developer Guide.
This PR corresponds to an issue that the Etaoin maintainers have agreed to address.
This PR contains test(s) to protect against future regressions
I have updated CHANGELOG.adoc with a description of the addressed issue.